Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: add limited_staff to allowedRoles [BB-7834] #2

Conversation

0x29a
Copy link
Member

@0x29a 0x29a commented Sep 18, 2023

Backported from here.

@Agrendalath
Copy link
Member

Agrendalath commented Oct 5, 2023

@0x29a, the change looks good to me, but I need to check something first, so I'll review this next week.

We cannot use the opencraft-release/palm.1 for this client because it uses @edx/frontend-platform v2.5.0, incompatible with a custom header component in Palm. Therefore, I created the opencraft-release/palm.1-echo branch for them, which includes ae1702d (that bumps @edx/frontend-platform version from 2.5.0 to ^4.2.0). Our opencraft-release/palm.1 does not have these changes, but it contains the cherry-picked 3644172. Therefore, I created opencraft-release/palm.1-echo-2, which includes all commits up to 3be81e0. I'll be deploying it to the client instance to test it. Then, I'd like to rebase our opencraft-release/palm.1 branch onto it.

Could it cause any trouble for the other client that uses this repo?

@0x29a
Copy link
Member Author

0x29a commented Oct 9, 2023

Could it cause any trouble for the other client that uses this repo?

@Agrendalath, I'm not that involved in the frontend development for this client, but I don't think it has any customizations that would conflict with the changes you're going to add.

@navinkarkera, do you know if bumping @edx/frontend-platform version from 2.5.0 to ^4.2.0 (ae1702d) in the gradebook MFE can break something for ASU?

@navinkarkera
Copy link
Member

@0x29a Unfortunately, I don't know.

@Agrendalath
Copy link
Member

cc: @viadanna, as the client owner ^

@viadanna
Copy link
Member

viadanna commented Oct 11, 2023

I don't see a problem 👍

@Agrendalath Agrendalath changed the base branch from opencraft-release/palm.1-asu-backup to opencraft-release/palm.1 October 16, 2023 20:51
@Agrendalath Agrendalath force-pushed the 0x29a/bb7834/fix-gradebook-access-for-limited-staff-palm branch from 168cf24 to 25bdb04 Compare October 16, 2023 20:57
Copy link
Member

@Agrendalath Agrendalath left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

  • I tested this: checked that the limited staff role can access gradebook
  • I read through the code
  • I checked for accessibility issues: n/a
  • Includes documentation: n/a
  • I made sure any change in configuration variables is reflected in the corresponding client's configuration-secure repository: n/a

@Agrendalath
Copy link
Member

I replaced the branches as described above. The original opencraft-release/palm.1 is now opencraft-release/palm.1-asu-backup.

@Agrendalath Agrendalath merged commit cbb32d1 into opencraft-release/palm.1 Oct 16, 2023
0 of 3 checks passed
@Agrendalath Agrendalath deleted the 0x29a/bb7834/fix-gradebook-access-for-limited-staff-palm branch October 16, 2023 21:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants